Conversation
The git plugin's FileContent method was cloning the repo on every call. This adds caching using filecache (matching the github plugin pattern), with TTL controlled by DEVBOX_X_GITHUB_PLUGIN_CACHE_TTL env var. Includes tests for cache hit, TTL expiry, and invalid TTL handling.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds file-based caching for gitPlugin.FileContent so git+https plugin repos aren’t cloned on every read, with cache TTL controlled by DEVBOX_X_GITHUB_PLUGIN_CACHE_TTL.
Changes:
- Add a
filecache-backed cache for git pluginFileContent, with TTL read fromDEVBOX_X_GITHUB_PLUGIN_CACHE_TTL. - Add tests covering cache hit behavior, TTL expiry behavior, and invalid TTL parsing.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| internal/plugin/git.go | Introduces filecache for FileContent and reads TTL from env var. |
| internal/plugin/git_test.go | Adds integration-style tests using a local bare git repo to validate cache/TTL behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: John Lago <750845+Lagoja@users.noreply.github.com>
Use ttl.String() instead of raw env var in cache keys to avoid path-separator injection and trailing slashes. Add t.Skip guard in setupLocalGitRepo for CI environments without git. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ipince
approved these changes
Mar 24, 2026
Lagoja
deleted the
jl/github-cache-backoff-env-variable-not-working-for-githttps
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When we implemented the new git+https plugins feature in the last release, we did not configure it to use the DEVBOX_X_GITHUB_PLUGIN_CACHE_TTL env var to set TTL for the plugin. The git plugin's FileContent method was cloning the plugin repo on every call. This adds caching using filecache (matching the github plugin pattern), with TTL controlled by DEVBOX_X_GITHUB_PLUGIN_CACHE_TTL env var.
How was it tested?
Added 3 tests:
Cache hit (TestGitPluginFileContentCache) — Clones from a local bare repo, then deletes the repo on disk and calls FileContent again. The second call succeeds, proving it’s served from cache rather than attempting a fresh clone.
TTL expiry via env var (TestGitPluginFileContentCacheRespectsEnvVar) — Sets DEVBOX_X_GITHUB_PLUGIN_CACHE_TTL=1ns so entries expire immediately, clones once, deletes the repo, then calls FileContent again. The second call fails because the expired cache entry forces a re-clone against the now-deleted repo. This proves the env var controls cache lifetime.
Invalid TTL (TestGitPluginFileContentCacheInvalidTTL) — Sets the env var to "not-a-duration" and verifies FileContent returns a parse error before attempting any clone.
Will also test functionality via a dev release.
Is this backwards compatible?
Yes